-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Bézier handles to path masks, so the control points can be moved independently #17204
base: master
Are you sure you want to change the base?
Conversation
No, you shouldn't touch what is related to translations. POT file is updated separately, contributors do not have to do this in PR. |
OK, I removed my localization changes. Do I have to submit them somewhere else separately? |
@marc-fouquet : Yes, you can simply make another (or better multiple PR to eventually discuss the translation with people speaking the target language without holding the other PR for being merged) for the translation work. TIA. |
I have submitted one extra PR for this (mainly because of the .pot-file, which is the prerequisite for the localization change, so I was not sure what to do with it in PRs that are separated by language). |
Two things, when using Also I see a small glitch on the feather which gets a little "corner" as soon as the lines are not aligned. See the feather red line over the clouds: |
The deadline for DT 5.0 is December, right? I will look into this some more and try to find improvements, but I can't promise that I will be fast. Regarding #1: I initially wanted to implement something that I think would have felt even more natural to me - basically the current behaviour, BUT with the added possibility to press or release the shift-key on the fly while already dragging the control point. I did not implement this, because in So far I wanted to keep my changes minimal. I did not yet research, where Regarding #2: This is another behaviour that was present before, but gets amplified after my change. I will have a look if I can improve this. One more thing that I don't like, but which was also present before: With weirder shapes, the feathering control points are not located on the feathering border and also not visually connected to their corresponding point on the regular border. Maybe I could just draw a line to connect the border point with the feather point, now that the perpendicular "Bézier handle" is gone. Another thing that I thought about was refactoring the code. It is unnecessarily hard to understand, with unclear variable names, a lack of comments, functions that are 100s of lines of code, no unit tests. Besides time, the main problem here is that I still don't fully understand everything, so I am afraid of breaking things. |
Yes, no urgency. Sorry, I just edited my first paragraph. I meant when edited after WITHOUT the shift modifier. |
I have worked on this some more. My current code is in need of some cleanup before it can be merged, but I won't have time in the next 2 weeks, so there will be a further delay. |
Hi Marc, thanks for your work I check your branch and report. |
The current controls:
I have added an optimization step to make the connections between segments smoother. However, this can't be used on all connections. I spent the most effort on ensuring that the feathering border behaves well, as long as the user acts somewhat reasonably. However, users have a lot of freedom to shoot themselves in the foot. Especially large feathering borders, large changes in the feathering borders, and overlapping curves are cases that may break the algorithm. In the worst case, it is possible to build a curve where some feathering control points are no longer reachable with the mouse, so the user might have to delete the shape and start over. However, this was also possible before. |
Great work! All is working fine on my side now. I still have a point to discuss:
For me we really need to find a way for this (or the opposite - that is a modifier to reset the angle) as it is not really user friendly as it is now. Adjusting the angle takes time and a simple click make you loose everything. |
My proposal:
So following is not needed:
@marc-fouquet : Is that ok ? |
TBH, I am confused by your replies. Maybe you looked at the wrong thing? The git branch associated with this PR (marc-fouquet:path_bezier_new) is unchanged since July. I intend to submit the new code here, but I need to work on cleaning it up first. Only https://github.com/marc-fouquet/darktable/tree/path_bezier_new_debug_2024-08-02 has the (messy) new code. There may also have been a misunderstanding regarding the controls. In the new version when dragging a control point without any modifier, the angle and the distance ratio are preserved. This is what I meant to say with my last post. So I don't see where "a simple click make you loose everything". |
@marc-fouquet : Sorry, yes probably a misunderstanding on my side! |
…d independently Includes changes to the way the feathering border is drawn for paths. The border could already be bugged in rare cases before. With the added freedom of moving the control points individually, broken feathering borders were much too easy to produce. Includes one changed UI tooltip in English, German, French, Italian.
Maybe one point to clarify. I meant if you click on a bezier control point the other gets align instead of keeping the current angle. This is important if what you want to do is just adjust the actual bezier controls keeping the current angle (quite common action I would say). Hope this clarifies this point, let me know. |
… and documented the code.
daddbd2
to
70e83e5
Compare
Hmmm. This was the behaviour in my very first version, but should have been different already in the code on my temporary branch. Anyway, I have updated the branch for this PR, please check this version. I think this version is ready to merge. The feature works and I also did some code cleanup on the parts that I worked with. |
Patch for issue #14236 .
Replaces my previous PR #17044 .
Includes changes to the way the feathering border is drawn for paths. The border could already be bugged in rare cases before. With the added freedom of moving the control points individually, broken feathering borders were much too easy to produce.
Includes one changed UI tooltip in English, German, French, Italian.